bugfix(contain): Restore retail compatibility after crash fix in OpenContain::killAllContained by Caball009 · Pull Request #2439 · TheSuperHackers/GeneralsGameCode

Caball009 · 2026-03-11T08:51:11Z

Closes Restore retail compatibility after crash fix in OpenContain::killAllContained #2215
Follow up for [ZH] Fix crash in OpenContain::killAllContained() when killed occupants kill the host container #921

The way the crash fix was implemented in #921 assumes that the logic behavior doesn't change if m_containList / m_containListSize is already empty with the first iteration. This is not the case in the attached replay in #2215.

If m_containList / m_containListSize is empty, it'll make garrisoned civilian buildings unoccupied on the first iteration instead of the last iteration, and the GLA Demolition upgrade would cause damage to the building for each occupant instead of just the last one when hit with a neutron shell.

garrisoned_gla_demolition_upgrade_neutron_shell_incorrect.mp4

garrisoned_gla_demolition_upgrade_neutron_shell_correct.mp4

Pay attention to the health of the civilian building. In the correct version, only the detonation of the last GLA RPG Trooper causes damage instead of every one.

TODO:

Polish comments.

greptile-apps · 2026-03-11T08:54:57Z

Greptile Summary

This PR restores retail-compatible behavior in OpenContain::killAllContained() that was inadvertently broken by the crash fix in #921. The prior fix pre-drained m_containList into a temporary list before iterating, which meant downstream callbacks (e.g. the garrisoned-infantry team-ownership logic) could no longer observe the live list state during iteration, causing civilian buildings to prematurely become unoccupied and the GLA Demolition upgrade to deal per-occupant damage instead of only on the last one.\n\nKey changes:\n- killAllContained() now manually erases each rider from m_containList and decrements m_containListSize before calling onRemoving / onRemovedFrom / kill(), so callbacks always see an accurate live list.\n- After each rider->kill(), the iterator is unconditionally reset to m_containList.begin() to handle any list mutations that kill() may have triggered (including the original recursive-drain crash scenario from #921).\n- A DEBUG_ASSERTCRASH + defensive m_containList.clear() / m_containListSize = 0 are added after the loop as a safety net.\n- The recursive-kill-crash scenario from #921 (GLA Terrorist inside a GLA Technical) remains safe: the first kill() drains the rest of the list recursively, so when control returns the iterator reset lands on end() and the outer loop exits cleanly.

Confidence Score: 5/5

Safe to merge — the logic is sound, the recursive-crash scenario from #921 is handled, and retail compatibility is restored.

No P0 or P1 issues found. The live-iteration approach correctly preserves the list state that downstream callbacks depend on. The begin()-reset after each kill() is the established pattern already used in harmAndForceExitAllContained(). The final clear()/size=0 is a harmless safety net. All remaining observations are P2 or lower and do not block merge.

No files require special attention.

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp	Rewrites killAllContained() to keep m_containList live during iteration instead of pre-draining it, fixing garrisoned-building team ownership and GLA Demolition damage behavior while still safely handling recursive kill() callbacks by resetting the iterator to begin() after each kill.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["killAllContained() entry"] --> B["it = m_containList.begin()"]
    B --> C{"it != end?"}
    C -- No --> H["DEBUG_ASSERTCRASH list empty"]
    H --> I["clear() / size=0 safety net"]
    I --> Z["return"]
    C -- Yes --> D["rider = *it"]
    D --> E{"rider != null?"}
    E -- No --> F["++it"] --> C
    E -- Yes --> G["erase(it), --m_containListSize"]
    G --> J["onRemoving(rider)"]
    J --> K["rider->onRemovedFrom(container)"]
    K --> L["rider->kill()"]
    L --> M{"kill() recursively\ndrained the list?"}
    M -- Yes --> N["it = begin() == end()"]
    M -- No --> O["it = begin()\nnext element"]
    N --> C
    O --> C

_{Reviews (4): Last reviewed commit: "Tweaked comments." | Re-trigger Greptile}

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp

…OpenContain::killAllContained.

Mauller · 2026-03-11T17:56:31Z

Does this still prevent the crashing with tech terrors?

Caball009 · 2026-03-11T17:59:57Z

Does this still prevent the crashing with tech terrors?

Yes, the original crash is still fixed. I made sure of that, and also created a reproduction of it in the crash fix PR.

Skyaero42

This looks correct to me.

Mauller

Looks good to me too

xezon

Logic looks plausible. Comments need polishing.

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp

Caball009 · 2026-03-20T16:32:27Z

Updated the comments.

I got rid of the old 'Neutron Shells + GLA Technical containing GLA Terrorists' issue reproduction description that seems too detailed to me.

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp

Caball009 added Bug Something is not working right, typically is user facing Major Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour ThisProject The issue was introduced by this project, or this task is specific to this project labels Mar 11, 2026

greptile-apps bot reviewed Mar 11, 2026

View reviewed changes

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp Outdated Show resolved Hide resolved

bugfix(opencontain): Restore retail compatibility after crash fix in …

341c271

…OpenContain::killAllContained.

Caball009 force-pushed the fix_OpenContain_killAllContained branch from 04af4f6 to 341c271 Compare March 11, 2026 08:57

Caball009 changed the title ~~bugfix(opencontain): Restore retail compatibility after crash fix in OpenContain::killAllContained~~ bugfix(contain): Restore retail compatibility after crash fix in OpenContain::killAllContained Mar 11, 2026

Skyaero42 approved these changes Mar 11, 2026

View reviewed changes

Mauller approved these changes Mar 11, 2026

View reviewed changes

xezon approved these changes Mar 11, 2026

View reviewed changes

Caball009 added 2 commits March 20, 2026 17:20

Merge branch 'main' into fix_OpenContain_killAllContained

cac2a0c

Updated comments.

4f79b64

xezon reviewed Mar 23, 2026

View reviewed changes

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp Outdated Show resolved Hide resolved

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp Outdated Show resolved Hide resolved

Tweaked comments.

a0aa05b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bugfix(contain): Restore retail compatibility after crash fix in OpenContain::killAllContained#2439

bugfix(contain): Restore retail compatibility after crash fix in OpenContain::killAllContained#2439
Caball009 wants to merge 4 commits intoTheSuperHackers:mainfrom
Caball009:fix_OpenContain_killAllContained

Caball009 commented Mar 11, 2026 •

edited

Loading

Uh oh!

greptile-apps bot commented Mar 11, 2026 •

edited

Loading

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Mauller commented Mar 11, 2026

Uh oh!

Caball009 commented Mar 11, 2026

Uh oh!

Skyaero42 left a comment

Uh oh!

Mauller left a comment

Uh oh!

xezon left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Caball009 commented Mar 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Caball009 commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Mauller commented Mar 11, 2026

Uh oh!

Caball009 commented Mar 11, 2026

Uh oh!

Skyaero42 left a comment

Choose a reason for hiding this comment

Uh oh!

Mauller left a comment

Choose a reason for hiding this comment

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Caball009 commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Caball009 commented Mar 11, 2026 •

edited

Loading

greptile-apps bot commented Mar 11, 2026 •

edited

Loading

Caball009 commented Mar 20, 2026 •

edited

Loading